upgrade: keyed package upgrade for Solid 2.0#910
Conversation
🦋 Changeset detectedLatest commit: 8526767 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/keyed/src/index.ts`:
- Around line 294-300: The memo used for Rerun is inadvertently tracking any
signals read when evaluating props.children, causing Rerun to subscribe to
child-internal dependencies; to fix, evaluate the child without tracking by
wrapping the child invocation in an untracked context (e.g., Solid's untrack) so
only props.on (and getKey) control reruns—update the anonymous memo callback
that calls getKey and executes props.children to call the child inside
untrack(() => ...) so child signal reads do not become dependencies of the Rerun
memo.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a2da682b-e28f-4da0-86aa-b84f06df1f05
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
.changeset/keyed-solid2-migration.mdpackages/keyed/package.jsonpackages/keyed/src/index.tspackages/keyed/test/index.test.tsx
| () => { | ||
| const currentKey = getKey(); | ||
| const child = props.children; | ||
| return typeof child === "function" && child.length > 0 ? (child as any)(a, b) : child; | ||
| }), | ||
| const el = | ||
| typeof child === "function" && (child as Function).length > 0 | ||
| ? (child as any)(currentKey, prevKey) | ||
| : child; |
There was a problem hiding this comment.
Avoid tracking child-internal dependencies in Rerun memo.
children is executed in a tracked createMemo context, so any signal reads during child evaluation can subscribe Rerun to dependencies unrelated to props.on. That changes rerun semantics versus the old on(...) behavior.
Proposed fix
return createMemo(
() => {
const currentKey = getKey();
const child = props.children;
- const el =
- typeof child === "function" && (child as Function).length > 0
- ? (child as any)(currentKey, prevKey)
- : child;
+ const el = untrack(() =>
+ typeof child === "function" && (child as Function).length > 0
+ ? (child as any)(currentKey, prevKey)
+ : child,
+ );
prevKey = currentKey;
return el;
},
{ equals: false },
) as unknown as JSX.Element;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/keyed/src/index.ts` around lines 294 - 300, The memo used for Rerun
is inadvertently tracking any signals read when evaluating props.children,
causing Rerun to subscribe to child-internal dependencies; to fix, evaluate the
child without tracking by wrapping the child invocation in an untracked context
(e.g., Solid's untrack) so only props.on (and getKey) control reruns—update the
anonymous memo callback that calls getKey and executes props.children to call
the child inside untrack(() => ...) so child signal reads do not become
dependencies of the Rerun memo.
1 participant
src/index.ts`
isServerandJSXtype now imported from@solidjs/webonhelper (removed in 2.0) andAccessorArray(removed in 2.0)addNewItem: addedINTERNAL_OPTIONS(ownedWrite: true) to bothcreateSignalcalls for item and index, satisfying Solid 2.0's owned-scope write restriction; also addedas Exclude<T, Function>cast required by the newcreateSignaloverloadsRerun: rewrote without theonhelper — tracksprevKeymanually in acreateMemo({ equals: false });AccessorArray<S>replaced withAccessor<S>[]package.jsonsolid-jsand@solidjs/webto2.0.0-beta.13@solidjs/webas both dev and peer dependencyvitest -c ../../configs/vitest.config.solid2.ts(shared config, no localvitest.config.ts)test/index.test.tsxcreateRootto avoidSIGNAL_WRITE_IN_OWNED_SCOPEerrorssetList(() => [...])for Solid 2.0createEffectupdate pattern with reactive getters ({ get value() { return v().value; } },{ get i() { return i(); } }) — getters read signals synchronously without needing deferred effect appliescreateMemo(synchronous) instead ofcreateEffect(deferred apply)renderimported from@solidjs/web.changeset/keyed-solid2-migration.md— changeset created marking this as a major version bump.Summary by CodeRabbit
isServerandJSXhave changed to@solidjs/webReruncomponent props have been updated